Skip to content

SpecUtils_NODE_BINDINGS compiler flag added to compile the node binding. #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

balamuruganky
Copy link
Contributor

@balamuruganky balamuruganky commented Apr 7, 2025

@wcjohns : This pull request is the implementation of the node build by enabling the compiler flag. Please let me know, if you have any suggestion on this PR to merge into master branch. Thank you.

@balamuruganky
Copy link
Contributor Author

@wcjohns : Please share your thoughts on this pull request, which enables to compile the nodejs with cmake flag. Thank you.

@wcjohns
Copy link
Collaborator

wcjohns commented Jun 23, 2025

Hi @balamuruganky,

I'm so sorry - I somehow missed or dropped taking a better look at this!

let me test this out on the various OSes, as well as out of source builds, in the next few days (hopefully!).

If I read it right, it allows building the node bindings from the top-level directory (instead of cd'ing into bindings/node), and it installs node-addon-api and cmake-js for you? It also fixes position independent code maybe not being generated (but maybe this should just always be turned on for this library?), and fixes up an include directory.

I'm a little hesitant to have CMake install packages for you - I'm not totally sure why, since it is just locally (in the source directory) in this case. Let me think about this and get someone else's opinion.

Thanks for making this pull request, and again, I am really sorry about somehow dropping taking a look at this.
-will

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants